Install a separate driver instance for each test by willie-yao · Pull Request #201 · kubernetes-sigs/dra-example-driver

willie-yao · 2026-05-13T17:42:17Z

Each It block now installs its own DRA driver release with a unique driver name, and uninstalls it on teardown.

Fixes #171

Changes:

New test/e2e/driver_test.go with installDriver(ctx, DriverConfig) helper. Shells out to helm (already required by setup-e2e.sh); honors HELM_CHART_PATH env var.
deployManifest accepts a DriverConfig and substitutes gpu.example.com -> per-test driver name (and example.com/gpu -> per-test extended resource name) in demo manifests before applying.
Beyond helm --wait, polls explicitly for DeviceClass + ResourceSlices + webhook readiness before returning from install.
Per-test DeferCleanup LIFO: driver-log diagnostics -> helm uninstall -> namespace delete with foreground propagation + termination wait.
setup-e2e.sh no longer installs the driver. Cert-manager + kind cluster + image build stay (cluster-wide infra).
Webhook specs share one driver pinned to gpu.example.com and run Ordered, Serial so their static testdata stays valid.

willie-yao · 2026-05-13T17:46:29Z

+
+// verifyWebhook waits until the validating webhook is serving for the given
+// DeviceClass by creating a dry-run ResourceClaim until it succeeds.
+func verifyWebhook(ctx context.Context, deviceClassName string) {


This code is mostly moved from e2e_setup_test.go at line 107: https://github.com/willie-yao/dra-example-driver/blob/main/test/e2e/e2e_setup_test.go#L107

Changes: now takes a deviceClassNAme parameter, the test claim's Name is suffixed with it due to parallel webhook installs and added DeviceClass to error messages

willie-yao · 2026-05-13T17:48:08Z

+	driverPods, err := clientset.CoreV1().Pods(cfg.Namespace).List(ctx, metav1.ListOptions{
+		LabelSelector: driverPodSelector,
+	})
+	if err != nil {
+		fmt.Fprintf(GinkgoWriter, "Failed to list driver pods: %v\n", err)
+		return
+	}
+	for _, pod := range driverPods.Items {
+		for _, c := range pod.Spec.Containers {
+			stream, err := clientset.CoreV1().Pods(cfg.Namespace).GetLogs(pod.Name, &corev1.PodLogOptions{
+				Container: c.Name,
+				TailLines: &tailLines,
+			}).Stream(ctx)
+			if err != nil {
+				fmt.Fprintf(GinkgoWriter,
+					"Driver pod %s, container %s: failed to get logs: %v\n",
+					pod.Name, c.Name, err)
+				continue
+			}
+			buf := new(bytes.Buffer)
+			_, _ = io.Copy(buf, stream)
+			stream.Close()
+			fmt.Fprintf(GinkgoWriter,
+				"Driver pod %s, container %s (last %d lines):\n%s\n",
+				pod.Name, c.Name, tailLines, buf.String())
+		}
+	}


This is moved from e2e_setup_test.go on line 192: https://github.com/willie-yao/dra-example-driver/blob/main/test/e2e/e2e_setup_test.go#L192

willie-yao · 2026-05-13T17:48:26Z

/assign @nojnhuh

nojnhuh

Sorry for the annoying comments on the workspace/lint periphery here. I promise I'm starting to look at the main point of this PR.

nojnhuh · 2026-05-14T21:09:44Z

 	golangci-lint run ./...
+	cd test && golangci-lint run --build-tags=e2e ./...


This hides lint failures in the test module if any issues are found in the main module. Can we combine into one call?

Suggested change

golangci-lint run ./...

cd test && golangci-lint run --build-tags=e2e ./...

golangci-lint run ./...

golangci-lint run --build-tags=e2e ./... ./test/...

Tried combining it with golangci-lint run --build-tags=e2e ./... ./test/... but it silently doesn't lint the test module. It logs pattern ./test/...: directory prefix test does not contain main module or its selected dependencies and exits 0 with 0 issues. I'm thinking to do it by just creating another make target for test

.PHONY: lint lint-root lint-test lint: @$(MAKE) -k lint-root lint-test lint-root: golangci-lint run ./... lint-test: cd test && golangci-lint run --build-tags=e2e ./...

With a workspace set up, it seems to work for me: willie-yao/dra-example-driver@separate-driver-install...nojnhuh:dra-example-driver:workspace-lint

% make lint golangci-lint run --build-tags=e2e ./... ./test/... cmd/dra-example-webhook/main.go:183:1: Comment should end in a period (godot) // function ^ test/e2e/e2e_setup_test.go:54:1: Comment should end in a period (godot) // driverPodSelector finds kubelet plugin Pods within an installed driver's release namespace ^ 2 issues: * godot: 2 make: *** [Makefile:83: lint] Error 1

Ah that was completely my bad. I had v0.0.0 set before when testing something. Setting it to v0.2.1 fixed it! Thanks for checking this for me

nojnhuh · 2026-05-14T21:20:34Z

+	k8s.io/api v0.36.0
+	k8s.io/apimachinery v0.36.0
+	k8s.io/client-go v0.36.0
+	sigs.k8s.io/dra-example-driver v0.0.0


go mod tidy seems to want to put v0.2.1 here if this doesn't already exist. That should still ultimately resolve to the local version though and not that actual tagged version.

I confirmed locally that with replace => ../ present, v0.2.1 or any other version resolves to the local checkout. Should I change this to v0.2.1?

nojnhuh · 2026-05-15T19:43:06Z

+// runUpgradeOrInstall emulates `helm upgrade --install`: if a release with
+// the configured name already exists it is upgraded in place, otherwise a
+// fresh install is performed.
+func runUpgradeOrInstall(ctx context.Context, actionCfg *action.Configuration, install *action.Install, chrt *chart.Chart, values map[string]any) {


In practice, will this ever actually perform an upgrade of an existing Helm release? If we expect each test to have a separate instance of the driver, then I think we'd want to error if that driver already exists instead of upgrade and risk affecting a different test.

The script used helm upgrade --install before mostly to be copy-pasteable, but that was always expected to first be run on a fresh kind cluster without the driver installed where a plain helm install should have worked.

Ah you're right, this actually neds up hiding the errors and was leftover from trying to replicate helm upgrade --install. I'll remove it and use install.RunWithContext directly.

nojnhuh · 2026-05-15T19:49:45Z

+	actionCfg := newHelmActionConfig(cfg.Namespace, registryClient)
+
+	install := action.NewInstall(actionCfg)
+	install.ReleaseName = cfg.ReleaseName


Could we automatically generate a release name instead? That will make sure there aren't conflicts between tests, tests don't have to decide for themselves what the driver should be called, and we don't have to validate them up front.

nojnhuh · 2026-05-15T20:02:59Z

+	Expect(validation.IsDNS1123Label(cfg.ReleaseName)).To(BeEmpty(),
+		"DriverConfig.ReleaseName %q must be a valid DNS-1123 label", cfg.ReleaseName)
+	if cfg.Namespace == "" {
+		cfg.Namespace = cfg.ReleaseName


Like the release name, can we generate a new, separate namespace for each instance of the driver?

nojnhuh · 2026-05-15T20:07:03Z

+	// maxDriverNameLen is the longest driver name that fits within Linux's
+	// 108-byte UNIX_PATH_MAX after the kubelet appends its registrar socket
+	// prefix and per-pod UID suffix.
+	maxDriverNameLen = 28


Can you expand on how this value was calculated?

TBH copilot came up with this and I don't think it's necessary for the E2E test since we auto-generate default driver names that won't go over the limit anyways. I'll go ahead and remove it

nojnhuh · 2026-05-15T20:13:16Z

Does it simplify things if we construct these ResourceClaims inline vs. reading them from a file?

nojnhuh · 2026-05-15T20:25:20Z

+// with the per-test driver name and (when set) extended resource name.
+func substituteDriverIdentifiers(raw string, drv DriverConfig) string {
+	GinkgoHelper()
+	out := strings.ReplaceAll(raw, defaultDeviceClassName, drv.DriverName)


Eventually, the manifests will be referencing other drivers in addition to gpu.example.com. Some manifests may even rely on multiple separate instances of the driver running different profiles. Not a blocker for this PR, but we should find a better way to handle dynamic driver names.

nojnhuh · 2026-05-18T19:22:23Z

FYI @willie-yao A couple of dependabot PRs merged which conflict with this PR.

nojnhuh

One more nit, otherwise LGTM.

Could you also check that all the mod/work/sum files are up to date?

go mod tidy && go -C test mod tidy
go work sync
go mod tidy && go -C test mod tidy # again

nojnhuh · 2026-05-18T20:52:42Z

+	Expect(cfg.ReleaseName).To(BeEmpty(),
+		"DriverConfig.ReleaseName is populated by installDriver; callers must not set it")
+	Expect(cfg.Namespace).To(BeEmpty(),
+		"DriverConfig.Namespace is populated by installDriver; callers must not set it")


Making these available to tweak and then checking to make sure they weren't tweaked isn't ideal. Could we set this in a separate local variable in installDriver and pass that around separately from the DriverConfig?

nojnhuh · 2026-05-18T21:41:17Z

Could you also check that all the mod/work/sum files are up to date?
go mod tidy && go -C test mod tidy
go work sync
go mod tidy && go -C test mod tidy # again

Having written this out, this sounds kind of insane and I'm open to the possibility that separate modules is going to be more pain going forward than it's worth. We could try sticking with one module for now and see if anybody runs into any issues. Splitting things up later will probably be easier than putting them back together.

willie-yao · 2026-05-18T21:43:33Z

Having written this out, this sounds kind of insane and I'm open to the possibility that separate modules is going to be more pain going forward than it's worth. We could try sticking with one module for now and see if anybody runs into any issues. Splitting things up later will probably be easier than putting them back together.

Yeah you're right. I noticed the go.work and go.sum files kept going out of date even when doing go mod tidy... I can go ahead and refactor into one module if you think that's a better way to go about things.

nojnhuh · 2026-05-18T22:50:28Z

Having written this out, this sounds kind of insane and I'm open to the possibility that separate modules is going to be more pain going forward than it's worth. We could try sticking with one module for now and see if anybody runs into any issues. Splitting things up later will probably be easier than putting them back together.

Yeah you're right. I noticed the go.work and go.sum files kept going out of date even when doing go mod tidy... I can go ahead and refactor into one module if you think that's a better way to go about things.

Yeah, I'm thinking let's go back to one module for now. Trying it here was still a worthwhile experiment nonetheless.

…netes-sigs#201)

nojnhuh

/lgtm
/approve

/hold in case we want to try to use Helm v4

nojnhuh · 2026-05-19T19:26:37Z

 	github.com/stretchr/testify v1.11.1
 	github.com/urfave/cli/v2 v2.27.7
 	google.golang.org/grpc v1.81.1
+	helm.sh/helm/v3 v3.21.0


There's a v4 for Helm which I don't think has too many changes from v3. Can we use that here? If it turns out to be more work, let's file an issue to update this since I'm not sure how long v3 will get security updates.

k8s-ci-robot · 2026-05-20T15:33:09Z

LGTM label has been added.

Details

Git tree hash: 4030de44291eece73f23b4bab4d9fda21c399085

k8s-ci-robot · 2026-05-20T15:33:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nojnhuh, willie-yao

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [nojnhuh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

nojnhuh · 2026-05-20T15:37:57Z

@willie-yao Could you also please squash?

Signed-off-by: William Yao <william2000yao@gmail.com>

nojnhuh · 2026-05-20T16:33:58Z

/lgtm

k8s-ci-robot · 2026-05-20T16:34:06Z

LGTM label has been added.

Details

Git tree hash: 378fbf454ad4408d9c7d8b34703fe2bd4383b985

willie-yao · 2026-05-20T16:38:19Z

Added helm v4
/hold cancel

github-project-automation Bot added this to Dynamic Resource Allocation May 13, 2026

github-project-automation Bot moved this to 🆕 New in Dynamic Resource Allocation May 13, 2026

k8s-ci-robot requested review from byako and pohly May 13, 2026 17:42

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 13, 2026

willie-yao commented May 13, 2026

View reviewed changes

k8s-ci-robot assigned nojnhuh May 13, 2026

nojnhuh reviewed May 13, 2026

View reviewed changes

Comment thread test/e2e/driver_test.go Outdated

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 13, 2026

willie-yao commented May 13, 2026

View reviewed changes

Comment thread .github/workflows/lint.yaml Outdated

nojnhuh reviewed May 13, 2026

View reviewed changes

Comment thread test/go.mod Outdated

nojnhuh moved this from 🆕 New to 👀 In review in Dynamic Resource Allocation May 13, 2026

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 13, 2026

harche mentioned this pull request May 14, 2026

Add KEP-4680 device health reporting with message field support #127

Draft

nojnhuh reviewed May 14, 2026

View reviewed changes

willie-yao force-pushed the separate-driver-install branch from e5f08a6 to 27b36c3 Compare May 15, 2026 17:17

nojnhuh reviewed May 15, 2026

View reviewed changes

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 18, 2026

willie-yao force-pushed the separate-driver-install branch from aa4b2eb to 833008d Compare May 18, 2026 20:05

nojnhuh reviewed May 18, 2026

View reviewed changes

willie-yao force-pushed the separate-driver-install branch from 7e93526 to c6421b6 Compare May 18, 2026 21:58

willie-yao requested a review from nojnhuh May 19, 2026 16:50

willie-yao added a commit to willie-yao/dra-example-driver that referenced this pull request May 19, 2026

Install a separate driver instance for each test (squashed from kuber…

229dae9

…netes-sigs#201)

willie-yao mentioned this pull request May 19, 2026

Add example for native resource requests #203

Open

nojnhuh approved these changes May 20, 2026

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 20, 2026

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 20, 2026

willie-yao force-pushed the separate-driver-install branch from e740038 to a78fddd Compare May 20, 2026 16:14

nojnhuh mentioned this pull request May 20, 2026

feat: Add DRA Device Binding Conditions (KEP-5007) #199

Open

Install a separate driver instance for each test

a85fb42

Signed-off-by: William Yao <william2000yao@gmail.com>

willie-yao force-pushed the separate-driver-install branch from a78fddd to a85fb42 Compare May 20, 2026 16:30

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2026

k8s-ci-robot requested a review from nojnhuh May 20, 2026 16:30

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2026

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 20, 2026

k8s-ci-robot merged commit e94a45a into kubernetes-sigs:main May 20, 2026
6 checks passed

		golangci-lint run ./...
		cd test && golangci-lint run --build-tags=e2e ./...

Conversation

willie-yao commented May 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willie-yao commented May 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nojnhuh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nojnhuh commented May 18, 2026

Uh oh!

nojnhuh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nojnhuh commented May 18, 2026

Uh oh!

willie-yao commented May 18, 2026

Uh oh!

nojnhuh commented May 18, 2026

Uh oh!

nojnhuh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented May 20, 2026

Uh oh!

k8s-ci-robot commented May 20, 2026

Uh oh!

nojnhuh commented May 20, 2026

Uh oh!

nojnhuh commented May 20, 2026

Uh oh!

k8s-ci-robot commented May 20, 2026

Uh oh!

willie-yao commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels